[RFC] Network policies #3611
[RFC] Network policies #3611
Conversation
k8s-ci-robot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aojea The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
cncf-cla: yes
|
/assign @BenTheElder |
k8s-ci-robot
added
the
size/L
| privileged: true | ||
| capabilities: | ||
| add: ["NET_ADMIN"] |
There was a problem hiding this comment.
doesn't privileged=true make the NET_ADMIN cap redundant, i think it should be privileged=false and only the required caps should be added.
There was a problem hiding this comment.
i guess this is from the original repo manifest;
https://github.com/kubernetes-sigs/kube-network-policies/blob/main/install.yaml#L85-L88
There was a problem hiding this comment.
yeah, I think I started to get granularity and gave up :)
There was a problem hiding this comment.
i think, it's fine to leave it privileged and remove the NET_ADMIN cap. maybe leave a TODO comment if someone wishes to experiment with caps and limit the access scope.
| @@ -0,0 +1,75 @@ | |||
| /* | |||
| Copyright 2019 The Kubernetes Authors. | |||
There was a problem hiding this comment.
| Copyright 2019 The Kubernetes Authors. | |
| Copyright 2024 The Kubernetes Authors. |
| // get the target node for this task | ||
| controlPlanes, err := nodeutils.ControlPlaneNodes(allNodes) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| node := controlPlanes[0] // kind expects at least one always |
There was a problem hiding this comment.
| // get the target node for this task | |
| controlPlanes, err := nodeutils.ControlPlaneNodes(allNodes) | |
| if err != nil { | |
| return err | |
| } | |
| node := controlPlanes[0] // kind expects at least one always | |
| // get the target node for this task | |
| node, err := nodeutils.BootstrapControlPlaneNode(allNodes) | |
| if err != nil { | |
| return err | |
| } |
| @@ -186,6 +186,8 @@ type Networking struct { | |||
| // If DisableDefaultCNI is true, kind will not install the default CNI setup. | |||
| // Instead the user should install their own CNI after creating the cluster. | |||
| DisableDefaultCNI bool `yaml:"disableDefaultCNI,omitempty" json:"disableDefaultCNI,omitempty"` | |||
| // If NetworkPolicies is true, kind will install the default Network Policy setup. | |||
| NetworkPolicies bool `yaml:"networkPolicies,omitempty" json:"networkPolicies,omitempty"` | |||
There was a problem hiding this comment.
given this is a bool i would put a verb in front of it, same as DisasbleDefaultCNI above.
e.g. InstallNetworkPolicies
|
@neolit123 the alternative is to ship it with kindnet #3612 and no API required |
i think making it not part of kindnet seems better to me. |
|
Making it part of kindnetd would make it easier for projects that are swapping out kindnet to exclude it, and for projects that aren't swapping out kindet ... I'm not sure why it would be a problem to include it in kindnet? I think that's the simpler approach, and consistent with e.g. ip masquerade just being built in to kindnet. In the future we may also look into if we can ship it in a single image / container by importing the controller or something like that (to share resources & patching better). |
Add support for network policies in KIND
/hold
Fixes: #842